Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Raise disabled during boot inside fallback #475

Merged
merged 1 commit into from
Mar 26, 2019
Merged

Raise disabled during boot inside fallback #475

merged 1 commit into from
Mar 26, 2019

Conversation

KaanOzkan
Copy link
Contributor

Noticed this code path being hit with locale == false in our application after disabling any i18n calls during boot phase. It makes sense to raise Disabled here as well and let the user know about their mistake.

@KaanOzkan KaanOzkan changed the title Raise disabled during boot inside fallback [WIP] Raise disabled during boot inside fallback Mar 22, 2019
@radar
Copy link
Collaborator

radar commented Mar 25, 2019

Hey, thanks for the PR. Can you show me an example of how to make it break first?

@KaanOzkan
Copy link
Contributor Author

KaanOzkan commented Mar 25, 2019 •
edited
Loading

I came across this break from a Rails method call.

class Dummy < ApplicationController
end

> Dummy.helpers
NoMethodError: undefined method `to_sym' for false:FalseClass
Did you mean?  to_s
from /.bundle/vendor/gems/i18n-1.6.0/lib/i18n/locale/fallbacks.rb:69:in `[]'

#helpers ends up calling i18n.fallback https://github.com/rails/rails/blob/master/actionview/lib/action_view/lookup_context.rb#L50 .

If it's called before initializing where locale is set to false we get the error. Actual fix will have to be in client code but figured we can give a better error message through raising Disabled.

@radar
Copy link
Collaborator

radar commented Mar 26, 2019

Seems like a good idea to me :) Can you please look into adding a test for this change too?

@KaanOzkan KaanOzkan changed the title [WIP] Raise disabled during boot inside fallback Raise disabled during boot inside fallback Mar 26, 2019
KaanOzkan
KaanOzkan commented Mar 26, 2019
View reviewed changes
test/locale/fallbacks_test.rb

# Test I18n::Disabled is raised correctly when locale is false during fallback

test "with locale equals false" do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you actually prefer this one to be in its own test class. I wasn't sure about adding it to this one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine :)

@radar radar merged commit ba83938 into ruby-i18n:master Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radar radar radar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KaanOzkan @radar